Skip to content

fix(registry): fail closed on supply paths - #7544

Merged
bokelley merged 15 commits into
mainfrom
vaduz
Sep 15, 2026
Merged

fix(registry): fail closed on supply paths#7544
bokelley merged 15 commits into
mainfrom
vaduz

Conversation

@bokelley

@bokelley bokelley commented Sep 15, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

  • fail closed when supply-path collection constraints, carriage bindings, authority envelopes, or provenance are absent or malformed
  • publish canonical semantics_version: "1" supply-path vectors and align schemas, OpenAPI, documentation, and verifier behavior
  • persist authority-scoped revocation holds and successful-fetch provenance with migration 597
  • bind authority admission, manifest rollback protection, cache adoption, and catalog projection in one transaction
  • reject malformed owner authorization selectors and preserve same-registrable-domain well-known redirects
  • stabilize the PostgreSQL private-ledger ordering regressions using database-visible lock barriers instead of scheduler sleeps

Testing

  • npm run precommit (1,063 repository tests plus lint guards and typecheck)
  • 130 focused supply-path service tests
  • 125 golden-vector, verifier, and crawler regression tests
  • 36/36 private-ledger PostgreSQL integration tests
  • both repaired PostgreSQL races repeated five times
  • 29 supply-path migration/cache PostgreSQL integration tests
  • migration validation, schema/compliance builds, protocol tarball, and canonical fixture checks

Risk and review focus

  • supply-path verification is intentionally stricter and fails closed at the authorization boundary
  • migration 597 is additive and leaves existing manifests untrusted until a successful provenance-aware refresh
  • protocol, security, and testing specialist re-reviews report no remaining blockers
  • the ledger stabilization changes test harness timing only; production timeout and fail-closed behavior are unchanged

Combines and rebases the work from #7513 and #7514 onto current main.

Open workspace in Conductor

bokelley and others added 15 commits September 15, 2026 10:44
@bokelley
fix(registry): fail closed on supply paths with canonical shared vectors
13badf4
@bokelley
fix(registry): preserve per-collection evidence fallback in bulk inqu…
6f94e9b 
…iries
@bokelley
fix(registry): persist supply-path authority and bind shared publishe…
7b2d394 
…r evidence
@bokelley
fix(registry): reject malformed provenance without losing held revoca…
e9316a2 
…tions
@bokelley
test(registry): prove malformed provenance preserves authority pins
35d527b
@bokelley
fix(supply-path): reject invalid authority envelopes before pin admis…
8ed940a 
…sion
@bokelley
fix(supply-path): refuse empty fragments in authority locations
40f1093
@bokelley @claude
fix(registry): renumber migration 591→597 to avoid collision with #7457
17e08c8 
Migration 591 was already reserved by #7457
(591_compliance_refresh_authenticated_credential.sql). Renumbering to
597, the next uncontested slot after a main/open-PR allocation audit
(main tops at 590; no open PRs claim 592–596).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017jbrQJh44D9RmYbz2yM6pY
@bokelley @claude
fix(registry): remove old migration 591 (superseded by 597)
3a2c35d 
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017jbrQJh44D9RmYbz2yM6pY
@bokelley
fix(db): reserve supply-path provenance migration 597
618c389
@bokelley
docs(db): record verified migration 588 through 597 composition
8807b37
@bokelley
style(registry): remove trailing contract blank line
dcd9af8
@bokelley
docs(db): refresh migration allocation audit after main advances
4f2cff1
@bokelley
test(addie): stabilize PostgreSQL ledger race barriers
afad2e9
@bokelley
fix(registry): harden supply-path cache admission
c51a7bb
@aao-secretariat aao-secretariat Bot added the ladon/needs-human-review Ladon has escalated this PR for human review. label Sep 15, 2026
aao-secretariat[bot]
aao-secretariat Bot reviewed Sep 15, 2026
View reviewed changes

@aao-secretariat aao-secretariat Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ladon verdict: Escalate to human review

Escalate — gated schema paths require human/CODEOWNERS approval, and a decision-class question is open.

This PR modifies published protocol source under static/schemas/source/** (adagents.json, core/collection.json), which sits behind the repo's hard, deterministic approval gate. gated_paths: true and review_decision: REVIEW_REQUIRED (not APPROVED), so per decision-table row 2 the outcome cannot be approve or plain comment — it must be escalate.

The reviewer found no Critical/High/Medium issues: schema↔docs coherence holds (new optional collection.publisher_domain and the revoked_publisher_domains durability rewrite match the managed-networks / collections docs), the changeset is present and correctly typed minor on additive-wire-shape grounds, no new undiscriminated oneOf, and no mutation of released dist/** artifacts. The largest file (supply-path-verifier.ts) is DoS-bounded with timeouts, byte caps, same-origin redirect policy, per-domain fail-closed admission, and transactional pin admission.

Two escalation triggers stand regardless:

  1. Gated schema paths under a hard approval gate with review not yet APPROVED — needs human/CODEOWNERS sign-off on static/schemas/source/adagents.json and static/schemas/source/core/collection.json.
  2. The reviewer explicitly flagged an open decision-class question: the fail-closed verdict-semantics tightening may be a Breaking-class change requiring human ratification. Per the repo's "Breaking-class changes are never auto-approved" hard gate, this is a human act and must be ratified on the thread — escalate unless review_decision is APPROVED.

No blocking code-level findings, but human review is mandatory before merge.

Why human review

  • Gated schema paths modified (static/schemas/source/adagents.json, static/schemas/source/core/collection.json) and review_decision is REVIEW_REQUIRED, not APPROVED — human/CODEOWNERS approval required (decision-table row 2)
  • Open decision-class question: fail-closed verdict-semantics tightening may be a Breaking-class change requiring human ratification per the repo's 'Breaking-class changes are never auto-approved' hard gate
  • This PR touches a path under a hard, non-overridable approval gate (static/schemas/source/adagents.json (modified) matches static/schemas/source/**; static/schemas/source/core/collection.json (modified) matches static/schemas/source/**) and the current GitHub review decision is 'REVIEW_REQUIRED', not APPROVED. This is a hard gate enforced in code — Ladon cannot auto-approve until a human/CODEOWNERS approval is recorded, regardless of how clean the diff is.

@bokelley
bokelley merged commit dc69e0b into main Sep 15, 2026
66 checks passed
@bokelley
bokelley deleted the vaduz branch September 15, 2026 19:43
This was referenced Sep 15, 2026
Merged
Merged
Closed
Merged
@bokelley conductor.build App

bokelley commented Sep 19, 2026

Copy link
Copy Markdown
Contributor Author

Downstream exact-head qualification of adcp-client#2908 found two residual fail-open cases in the merged verifier that are not represented in the canonical corpus:

  1. The host authorization path calls propertyScope() without first requiring hasPropertyAuthorizationEnvelope(), so a malformed host grant can still authorize.
  2. Top-level hostManifest.properties[] records are matched by property_id/publisher attribution without validating the required property envelope (property_type, name, and non-empty identifiers). This can produce verified_owner_sold from a record such as { property_id: "app" }.

The downstream implementation now fails closed for both cases and includes negative controls at adcp-client head da59d98a90ccab1a3f379ab45cc3e3fb8a52e0b7. A follow-up upstream patch and canonical vectors for malformed host grants plus malformed top-level properties would keep the registry and SDK semantics aligned.

This does not change the canonical provenance for #7544: downstream remains pinned to merged SHA dc69e0b2264b4c563f2175a48c515b7108dbb4f6 and corpus SHA-256 1a239d61209896aad843375d307650fb22dbfe7a8c73ebb900d8aec6d58abd31.

@bokelley

bokelley commented Sep 19, 2026

Copy link
Copy Markdown
Contributor Author

Both cases are confirmed in the merged verifier and patched on branch claude/7544-host-auth-property-envelope-guards (commit cd17449).

Issue 1 — host grant not checked by hasPropertyAuthorizationEnvelope

verifySupplyPath lines 395–411 (post-merge) call propertyScope(e, …) inside covered.find without first gating on hasPropertyAuthorizationEnvelope(e). A host grant with a malformed authorization envelope — e.g. authorization_type: "inline_properties" with a properties array whose entries lack name or identifiers — passes the record() filter in propertyScope and reaches resolveAgentProperties, whereas hasPropertyAuthorizationEnvelope's validInlineProperty would reject it. The owner-agent path already has this guard; the host path was missing it.

Fix: add if (!hasPropertyAuthorizationEnvelope(e)) return false; at the top of the covered.find callback, parallel to the existing owner-agent path.

Issue 2 — bare hostManifest.properties[] records matched in carriage and propertyScope

Properties were filtered only by publisher_domain attribution. A record like { property_id: "app" } (no property_type, name, identifiers) counted as a match in the carriage leg — setting carriage.ok = true — and was also included in the propertyScope property list for publisher_properties-type grants, letting selection_type: "all" resolve it. Together this allows verified_owner_sold from a bare property stub.

Fix: new hasPropertyEnvelope helper (property_type ∈ PROPERTY_TYPES, name: string, identifiers.length > 0) applied at all three property-filtering sites: carriage leg, propertyScope publisher-properties branch, and supplyPathAdsTxtPolicy.

Canonical vectors

Two negative controls added to static/compliance/source/test-vectors/supply-path/vectors.json:

  • host-grant-malformed-inline-property-envelope — host inline grant missing name/identifiers; expected host_authorization.failure: "property_scope_mismatch", state owner_attested
  • host-manifest-bare-property-record — host property { property_id: "hoststream_ctv" } only; expected owner_distribution_carriage.failure: "property_ids_unresolved", state owner_attested

Both align with the fail-closed semantics the downstream holds at adcp-client head da59d98.

Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@aao-secretariat aao-secretariat[bot] aao-secretariat[bot] left review comments

Assignees

No one assigned

Labels

ladon/needs-human-review Ladon has escalated this PR for human review.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@bokelley